fix(cli): discover resumable sessions from /resume - #3582

Reviewed at exact head e23405ce34db55480faf0f2a269cc2105935c8a2.

Coverage: how /resume discovers sessions, and the call chain from picking one to actually resuming. Not covered: wording.

Thanks for adding this entry point — the gap it targets is real. Two things need fixing before it does what it says, and both are reproducible on the ordinary user path.

[P2] onlyResumable filters on "attachable", not "has a safe boundary to resume"

packages/cli/src/pi-tui-runner.ts:2214-2215 selects entries with availability.get(session.id)?.available === true. But that availability comes from inspectSessionResumeAvailability at packages/cli/src/session-driver.ts:182-190, which only checks that session.cwd exists and is realpath-able. The Runtime Host implementation is the same shape, and for remote sessions it returns true for anything that has a cwd at all.

The authority for "is there something to resume" is elsewhere: the Host's turn.resume.query, where the Runtime plans a continuation only from that session's failed/cancelled inline run and otherwise reports resume_candidate_missing. The list path never calls it, and does not consult runningTurnIds or corrupted/unexecutable state either.

So an ordinary completed session, a session that is currently running, or one with no continuable run all appear in /resume as long as their working directory still exists. On remote, runtime-host-tui-command.ts:97-102 pins the scope to all while the new path applies no project predicate, so sessions from other projects are listed too. The same weak predicate also drives the startup hint at pi-tui-runner.ts:2282-2297, which will tell the user a plain attachable session "has an interrupted run".

Suggested direction: have a Runtime-owned resumability query return each session's real disposition, and filter here on current project, deleted workspace, running, and corrupted state — rather than reusing SessionResumeAvailability, which only answers cwd attachability.

[P2] Selecting from the picker does not actually resume

With no attached session, pi-tui-runner.ts:2134-2137 opens showSessionList({ onlyResumable: true }) and returns. The picker's onSelect at :2251-2260 calls goToSession(item.value), which on the idle path runs switchSession (:1649-1652) — and the comment right above goToSession says it plainly: /session is view navigation.

switchSession in runtime-host-session-driver.ts:471-524 validates cwd and the execution boundary, opens a subscription, and attaches a still-running root turn. It never calls turn.resume.query or turn.resume.start. The only driver path that starts a safe-boundary continuation is resumeLatest() at :325-349.

The user therefore picks a session, the picker closes as if the action succeeded, and the interrupted turn does not continue — they typically have to type /resume a second time so the current-session path reaches resumeLatest. That is the core function of the new entry point.

The new test at pi-tui-runner.test.ts:3265-3292 only asserts list text and filtering; it does not press Enter and assert that resumeLatest or a turn start follows, which is why this got through.

Suggested direction: make the selection path call a Runtime-owned switch-and-resume operation, or call resumeLatest once explicitly after a successful switchSession, keeping race/parked/failure outcomes visible.

Note on CI

check-runs on this head is total_count 0 — no run at all, which is neither green nor red. Even once the two items above are fixed, the gate needs terminal green on the new head.

Verification and limits

Changed-file Biome and git diff --check pass. A full CLI build could not be completed in our environment — pre-existing workspace drift unrelated to this PR blocked it — so no targeted-suite pass is claimed here; the findings above are established by reading the call chains at this exact head.

Reviewed at exact head f2c24a35. Two [P1]s, both inline. Publishing on behalf of a reviewer without write access here; the analysis is theirs, and I re-verified both against this head before posting.

Gate status is red, but not because of this PR. The test job's Build step fails on apps/desktop/src/main/__tests__/goal-services-adapter.test.ts:64,70,76 with TS2353: 'type' does not exist in type 'SessionChangedEvent'. That file is untouched here — this PR changes four packages/cli files. The root cause was already fixed on main by cded195 (#3642), so merging current main into this branch and re-running should clear it.

The direction is right: /resume should be able to open a picker when there is no current session, and startup should tell you an interrupted run exists. The two findings are both about the resume predicate being reused where an attachability predicate is what the caller needs.

Reviewed at 253fb50e595de0604c0c22b303fae1fa8023af82. Both of the [P1]s we raised earlier are fixed by this commit. No P0-P2 remain. Two gate blockers below, one of which is not yours to fix in code.

Correcting our own scope first

An earlier round of this review was working from a 252-file, +7405/-6516 diff. That is not what this PR changes. This branch was rebased onto main and replayed roughly twenty squash-merged commits; because squashing produces new SHAs, git cannot see them as shared history, so the merge-base is stuck at an old commit and every merge-base-relative view — the Files tab and the REST file list included — inflates accordingly. Those views are not wrong; they accurately report merge-base-to-head, which is simply not the same question as "what did the author change here".

The author-owned delta is one commit:

253fb50e fix(cli): separate session attach and resume discovery
  packages/cli/src/__tests__/pi-tui-runner.test.ts  +30
  packages/cli/src/pi-tui-runner.ts                 +37/-14
  packages/cli/src/runtime-host-session-driver.ts   +17/-17
  packages/cli/src/session-driver.ts                 +1
  4 files, +68 / -17

We are stating this because we got it wrong in this thread before stating it, and because the wrong number changed a conclusion further down.

Both prior [P1]s are closed

Attach availability no longer answers the resume-candidate question. getSessionResumeAvailability drops its host branch and returns inspectRuntimeHostSessionResumeAvailability unconditionally; the narrow turn.resume.query path moves into a new getSessionResumeCandidateAvailability. showSessionList picks by mode, and — this is the line that fixes the reported symptom — the selection handler changed from if (availability.get(item.value)?.available === false) return; to if (options.onlyResumable && availability.get(item.value)?.available === false) return;. /session no longer refuses to navigate to a session because a resume-candidate check said false. The commit subject says exactly this, and the code matches it.

The startup enumeration race is fixed at the mechanism, not by moving the timer. A memoised sessionListPromise now backs a shared listSessions(), so showSessionList and announceResumeAvailability join one in-flight enumeration instead of issuing two. announceResumeAvailability also returns early when the driver lacks getSessionResumeCandidateAvailability. The setTimeout(..., 0) is still there, and that is fine: the timer was never the defect — the concurrent duplicate enumeration was, and it is now memoised away.

One supporting argument from our earlier review should be retired rather than repeated: we noted that the availability map was built with a bare Promise.all(sessions.map(...)) while the sibling foreign-session branch had per-item containment. That asymmetry is gone — and specifically, this commit is what added the per-session try/catch. It is not that our evidence went stale; the author fixed it.

Gate: two blockers, both needing one push

1. Formatting. The test job fails at Check formatting on pi-tui-runner.ts around the new resume-availability expression. That step runs before the tests, which has a consequence worth naming: no test on this head has executed, so CI currently cannot tell you anything about the change itself. npm run format resolves it.

2. TS2304: Cannot find name 'decodeStoredMessage' in the package job — please do not hand-edit this. The symbol is in packages/storage/src/__tests__/codex-session-adapter.test.ts:267, which your commit does not touch. The branch replayed the older versions of #3562 and #3520 in the order that briefly broke main: the rename landed first, then an assertion still using the old symbol. main was repaired afterwards by #3656, but this branch replayed before that fix existed. Rebasing onto current main brings #3656 with it and clears this. Editing line 267 by hand would mean changing a file that is not part of your change.

The PR is also CONFLICTING, so a rebase is required regardless — one push can settle all three.

One thing to check on your side

The two review threads above show replies from you reading System.Collections.Hashtable[...]. That is a PowerShell object stringified by whatever tooling posted them, so no text of yours actually arrived. From the GitHub UI the threads look answered, which is why we are pointing it out rather than assuming you chose not to reply. If you meant to say something there, it will need reposting.

Correction to my previous review on this PR (5004256305), specifically to the scope paragraph.

I wrote that the author-owned delta is one commit, 4 files, +68/-17. That is wrong. It is three commits and 5 files.

9e24a005 fix(cli): discover resumable sessions from /resume            2 files, +63/-2
f2c24a35 fix(cli): resume the selected interrupted session             4 files, +25/-6
253fb50e fix(cli): separate session attach and resume discovery        4 files, +68/-17

Union: pi-tui-runner.ts, runtime-host-session-driver.ts, session-driver.ts, __tests__/pi-tui-runner.test.ts, __tests__/runtime-host-session-driver.test.ts. The +68/-17 I quoted is the tip commit alone, not this branch's contribution.

How I got it wrong is worth stating, because the previous review's whole first section was about scope. I listed the commits between the merge-base and the head, truncated the output, and read the top entry as the PR's own work. The other two author commits are further down that list, interleaved among the twenty-odd replayed main commits. A truncated observation reported as a fact — the same failure mode I was describing one paragraph earlier.

The reliable way to separate them is patch equivalence rather than reading a commit list:

git log --cherry-pick --right-only --no-merges origin/main...<head>

which returns exactly these three and marks the other nineteen as already present on main. Credit to a reviewer on our side for pointing out that subject-line matching is not sufficient here; this is what caught my error.

Nothing else in that review changes. Both [P1]s are still closed, and I verified that by reading the tip commit's actual diff — the removal of the host branch from getSessionResumeAvailability, the new getSessionResumeCandidateAvailability, the options.onlyResumable && guard on the selection handler, and the memoised sessionListPromise. The two gate blockers are unchanged: run npm run format, and rebase onto current main so that #3656 clears the TS2304 on a file that is not part of your change.

The practical difference for you is only in how much of this branch a reviewer should be reading: five files across three commits, not four files in one.

Correcting one sentence in 5004276704 — a tooling claim, not a conclusion about this PR.

I wrote that

git log --cherry-pick --right-only --no-merges origin/main...<head>

"returns exactly these three and marks the other nineteen as already present on main". It does not mark anything. --cherry-pick omits commits whose patch is already present on the other side; nothing in its output refers to them. The marking form is git cherry -v (+ / -) or --cherry-mark, which prints = for a patch-equivalent commit and > for one only on the right side. On this branch --cherry-mark gives 19 = and 3 >.

Two limits on that command that my wording also glossed over:

It produces a candidate classification, not a proof of ownership. A replayed commit rewritten during conflict resolution is no longer patch-equivalent and will appear as author work; conversely, a commit the author genuinely wrote that happens to be patch-identical to something already on main is omitted and silently disappears from their delta. The first direction costs extra reading, the second can hide real author work, and neither is detectable from the command's output alone.

--no-merges is --max-parents=1, so any delta introduced by a merge commit's own conflict resolution is excluded. That is harmless here — this branch has no merge commits, it was rebased — but the exclusion is silent, so it is worth stating rather than assuming.

None of this changes the facts in the previous correction, which an independent reviewer on our side re-derived from a fresh fetch: three author commits (9e24a005, f2c24a35, 253fb50e), five files in the union, none of them the Codex adapter test, and all three on this head's first-parent chain. The [P1] closures and the two gate items are likewise unaffected — still npm run format, and rebase onto current main so #3656 clears the TS2304.